fix(vcvalidator): reject Data Integrity proof with no verificationMethod - #895
Open
skdas20 wants to merge 1 commit into
Open
fix(vcvalidator): reject Data Integrity proof with no verificationMethod#895skdas20 wants to merge 1 commit into
skdas20 wants to merge 1 commit into
Conversation
With requireProof=false the JSON-LD signature is deliberately not checked, so a resolvable verificationMethod is the only remaining evidence about the credential. When verificationMethod was empty the resolution block was skipped entirely and verify() returned nil, accepting the credential with no verification performed and no error raised. Reject that case with AUT_SIGNATURE_MISSING, matching the two sibling 'nothing to verify' cases in the same function (no proof at all, and a proof carrying neither jwt nor proofValue). Fixes beckn#885 Signed-off-by: skdas20 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #885.
The bug
In
vcvalidator'sverify(), theproofValuebranch skips signature verification whenrequireProof=false(documented as "accept on expiry/revocation only") and falls back to confirming that theverificationMethodDID resolves. That fallback sits behind a non-empty guard:When
verificationMethodis empty the block is skipped entirely andverify()returnsnil— the credential is accepted having had no verification of any kind performed, and with no error raised to signal it.This is the same "nothing left to verify" situation as the two sibling cases in the same function —
cred.Proof == nil, and a proof carrying neitherjwtnorproofValue— both of which already reject withAUT_SIGNATURE_MISSING. This third case was silently accepted instead.Only reachable on deployments that explicitly set
requireProof=false;DefaultConfig()sets it totrue.The fix
Reject with
AUT_SIGNATURE_MISSINGwhenverificationMethodis absent, as suggested on the issue, so all three cases are classified consistently.Testing
Added
TestDataIntegrityProofWithoutVerificationMethod, covering the gap noted on the issue (no existing test exercisedrequireProof=falsewith an emptyverificationMethod).It fails on
mainwith exactly the reported symptom:That
nilis the defect — the credential passed. With the change the test passes, the rest of thevcvalidatorpackage is green, andgo vetis clean.